New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Regression: analytics selections malfunctioning #117
Regression: analytics selections malfunctioning #117
Conversation
So what is the difference between functions in |
One is a module and the other isn't. The bug had something to do with function definitions in js modules needing to be exported, but I don't know the fine details of it. So rather than exporting all function definitions in flexmeasures.js I think it is safer at this point to move out the recently introduced js content to a separate js module file, as I did in this PR. Otherwise I was worried about new side effects. Just to be clear, flexmeasures.js had not been a module type js file before, until we introduced the new (now to be separated) content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still have a template which should switch over to the module:
± grep -r flexmeasures.js flexmeasures
flexmeasures/ui/templates/views/sensors.html: import { subtract, thisMonth, lastNMonths } from "{{ url_for('flexmeasures_ui.static', filename='js/flexmeasures.js') }}";
Also, looking at that statement, it seems cleaner and more closer to the javascript module idea: import what you need. Shouldn't we do it the same way in base.html?
Other than that, I approve technically with this move.
I do, however, suggest better naming to move forward with this distinction (using modules has the main benefit to better organize the code):
- flexmeasures.js -> "ui-behaviours.js" (I actually want to factor out feature-specific code from here, like for the control page [lines 212-287], I'll make a ticket)
- flexmeasures-module.js -> "daterange-utils.js"
Good catch. The js module contents are actually not needed anywhere else currently, and I also prefer to import only what we need, so I'll stop importing our js module in our base.html. I'll also rename the module to "daterange-utils.js". Just a note: this regression was caused by 1a238ef. |
Fixed regression and started more targeted loading of javascript functions from our own js modules. * Create draft PR for #116 * Fix bug due to redefining older javascript content as a js module. * Changelog entry * More targeted loading of js functions from module Co-authored-by: Flix6x <Flix6x@users.noreply.github.com> Co-authored-by: F.N. Claessen <felix@seita.nl> Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com>
@meeseeksdev Hello |
Look at me, @Flix6x, I'm Mr. Meeseeks! |
closes #116